-
Notifications
You must be signed in to change notification settings - Fork 1k
Fix: not matched error code if arguments too few in RpcServer
#4070
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
would be great if you can make them use RpcMethodWithParams, but this pr is also good for me. |
| public static void ThrowIfTooFew(JArray @params, int requiredArgs, RpcError error) | ||
| { | ||
| if ((@params is null && requiredArgs > 0) || (@params is not null && @params.Count < requiredArgs)) | ||
| throw new RpcException(error.WithData($"Too few arguments. Expected at least {requiredArgs} but got {@params?.Count ?? 0}.")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think this type of exception messgae is too general, would be better to specify which is missing for dedicated rpc methods. but better than nothing.
| [RpcMethod] | ||
| protected internal virtual JToken InvokeFunction(JArray _params) | ||
| { | ||
| RpcException.ThrowIfTooFew(_params, 2, RpcError.InvalidParams); // ScriptHash, Operation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
params ? _params[2]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does params ? _params[2] mean?
I will do it |
5f93854 to
bb69f48
Compare
Description
If too few arugments, the error code shoud be InvalidParam, but got an unexpected exception info.
After fix:
Type of change
Checklist: